refactor: formalize C ABI codec registry via as_c_name/from_c_name#265
Merged
streamer45 merged 7 commits intomainfrom Apr 7, 2026
Merged
refactor: formalize C ABI codec registry via as_c_name/from_c_name#265streamer45 merged 7 commits intomainfrom
streamer45 merged 7 commits intomainfrom
Conversation
Centralizes codec name ↔ enum conversion in AudioCodec and VideoCodec, eliminating hardcoded string matching duplicated across 6 locations in the plugin SDK (conversions.rs, processor macro, source macro × input/output). Key changes: - Add AudioCodec::as_c_name()/from_c_name() and VideoCodec equivalents as the single source of truth for C ABI codec name strings. - Fix EncodedVideo to carry codec info via custom_type_id (same pattern as EncodedAudio). Previously mapped to PacketType::Binary with a TODO. Null custom_type_id falls back to Binary for backward compat. - Replace 4 duplicated match blocks in native_plugin_entry! and native_source_plugin_entry! macros with as_c_name() calls. - Add EncodedVideo custom_type_id handling to both macros. - Document the codec extensibility pattern in CPacketTypeInfo. - Add roundtrip tests for all codec name conversions and C ABI encoding. Adding a new codec now requires only two changes: 1. Add the variant to AudioCodec/VideoCodec 2. Add its name in as_c_name() and from_c_name() No API version bump needed — CPacketTypeInfo struct layout unchanged. BREAKING CHANGE: EncodedVideo pin declarations now carry codec info via custom_type_id instead of mapping to Binary. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![staging-devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/870844?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ec strings Add CodecName(CString) variant to CPacketTypeOwned and a codec_name_to_cstring() helper so packet_type_to_c derives its null-terminated codec names from as_c_name() instead of maintaining parallel hardcoded byte literals. This makes the doc comment claim true: as_c_name()/from_c_name() are now genuinely the only places codec-name strings live. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
- codec_name_to_cstring: use expect() instead of unwrap_or_default() to fail fast on programmer errors (matches lib.rs macro pattern) - Add tracing::warn! when EncodedVideo null custom_type_id falls back to Binary (aids debugging of new plugins vs backward compat) - Simplify map_err to forward original from_c_name() error messages - Add doc comments clarifying intentional field loss (bitstream_format, codec_private, profile, level) through C ABI pin-type declarations Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
- codec_name_to_cstring: use expect() instead of unwrap_or_default() to fail fast on programmer errors (matches lib.rs macro pattern) - Add tracing::warn! when EncodedVideo null custom_type_id falls back to Binary (aids debugging of new plugins vs backward compat) - Simplify map_err to forward original from_c_name() error messages - Add doc comments clarifying intentional field loss (bitstream_format, codec_private, profile, level) through C ABI pin-type declarations Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
- Make from_c_name strict (canonical lowercase only): remove 'avc'/'avc1' aliases from VideoCodec::from_c_name — serde aliases are for config deserialization, C ABI is a controlled interface with canonical names - Add tracing::warn! for EncodedAudio null custom_type_id fallback to Opus (matching the EncodedVideo pattern) - Make codec_name_to_cstring pub and replace 8 inline CString::new() .expect() sites in lib.rs macros — fully centralizes CString construction for codec names - Fix EncodedAudio doc comment: audio_codec -> custom_type_id - Replace alias roundtrip test with strict-mode rejection test Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Centralizes C ABI codec name handling into a single source of truth (
as_c_name()/from_c_name()onAudioCodec/VideoCodec), fixesEncodedVideoto carry codec metadata through the C ABI (was a TODO mapping toBinary), and eliminates duplicated codec-name logic across macros and conversions.Key changes:
AudioCodec::as_c_name()/from_c_name()andVideoCodecequivalents incrates/core/src/types.rs— single source of truth for codec ↔ C name mappingfrom_c_nameis strict: canonical lowercase only (no serde aliases like"avc"/"avc1")EncodedVideonow carries codec info viacustom_type_idinpacket_type_from_c/packet_type_to_c(was falling back toBinary)CPacketTypeOwned::CodecName(CString)eliminates duplicated byte literals inpacket_type_to_ccodec_name_to_cstring()madepuband used by all 8 macro sites inlib.rs— fully centralizesCStringconstruction withexpect()(fail-fast on programmer errors)tracing::warn!on bothEncodedVideoandEncodedAudionullcustom_type_idfallbacks (aids debugging vs backward compat)EncodedAudiodoc (audio_codec→custom_type_id)Adding a new codec now requires changes in exactly 2 methods (
as_c_name+from_c_name) instead of 6+ locations.Review & Testing Checklist for Human
from_c_namestrict mode is acceptable —"avc"/"avc1"no longer accepted in C ABI (serde aliases still work for config deserialization)EncodedVideobackward compat: nullcustom_type_id→Binaryfallback withtracing::warn!is appropriate for pre-codec-string pluginslib.rsto confirmcodec_name_to_cstringcalls look correct (4 macro blocks × 2 codec arms each)codec_name_to_cstringshould remainpubor if there's a way to scope it more narrowly (neededpubbecause macros expand in plugin crates)Notes
OpusAudiodiscriminant is preserved for backward compat with v1-v7 pluginsCPacketTypeInfostruct layout is unchanged —custom_type_idfield already existed, we're reusing it forEncodedVideoEncodedVideo, so no plugin-side breakagefrom_c_nameto get compile-time enforcement of the inverse mapping (noted in review, deferred)Link to Devin session: https://staging.itsdev.in/sessions/aff5f53f0854449fb15f91a1eef99f93
Requested by: @streamer45